Skip to content

Respond with 404 for invalid file references#7197

Merged
labkey-tchad merged 4 commits intorelease25.11-SNAPSHOTfrom
25.11_fb_invalidePipelinePath
Nov 13, 2025
Merged

Respond with 404 for invalid file references#7197
labkey-tchad merged 4 commits intorelease25.11-SNAPSHOTfrom
25.11_fb_invalidePipelinePath

Conversation

@labkey-tchad
Copy link
Member

@labkey-tchad labkey-tchad commented Nov 10, 2025 •
edited by labkey-jeckels
Loading

Rationale

Introduce InvalidPathReferenceException to signify a path we don't want to handle, either because it has invalid characters or it's trying to escape its parent.

The link crawler found this error when running on Windows due to the file system rejecting certain characters via File.toPath. Similar errors can be reproduced on Mac/Linux by attempting to reference a parent path (/home/specimen-importSpecimenData.view?path=..)

Stacktrace (Windows) 

java.nio.file.InvalidPathException: Illegal char <"> at index 84: D:\teamcity\work\cdd6dd8a02f96780\server\testAutomation\data\studies\Study001\study\">'>'"<\script><img src="x" onerror="alert('8(')">
	at java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
	at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
	at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
	at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
	at java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:232)
	at java.base/java.io.File.toPath(File.java:2387)
	at org.labkey.api.util.FileUtil.appendPath(FileUtil.java:832)
	at org.labkey.pipeline.api.PipeRootImpl._resolveRoot(PipeRootImpl.java:402)
	at org.labkey.pipeline.api.PipeRootImpl.resolvePath(PipeRootImpl.java:375)
	at org.labkey.pipeline.api.PipeRootImpl.resolvePath(PipeRootImpl.java:368)
	at org.labkey.api.pipeline.browse.PipelinePathForm.getValidatedFiles(PipelinePathForm.java:103)
	at org.labkey.api.pipeline.browse.PipelinePathForm.getValidatedFiles(PipelinePathForm.java:96)
	at org.labkey.specimen.actions.SpecimenController$ImportSpecimenDataAction.getView(SpecimenController.java:1086)

Stacktrace (Mac) 

java.nio.file.InvalidPathException: Path to parent not allowed: ../path
	at org.labkey.api.util.FileUtil.appendPath(FileUtil.java:829)
	at org.labkey.pipeline.api.PipeRootImpl._resolveRoot(PipeRootImpl.java:402)
	at org.labkey.pipeline.api.PipeRootImpl.resolvePath(PipeRootImpl.java:375)
	at org.labkey.pipeline.api.PipeRootImpl.resolvePath(PipeRootImpl.java:368)
	at org.labkey.api.pipeline.browse.PipelinePathForm.getValidatedFiles(PipelinePathForm.java:103)
	at org.labkey.api.pipeline.browse.PipelinePathForm.getValidatedFiles(PipelinePathForm.java:96)
	at org.labkey.specimen.actions.SpecimenController$ImportSpecimenDataAction.getView(SpecimenController.java:1086)
	at org.labkey.specimen.actions.SpecimenController$ImportSpecimenDataAction.getView(SpecimenController.java:1078)

Related Pull Requests

  • N/A

Changes

  • Introduce InvalidPathReferenceException
  • Report as a 404 with special logging
  • Update PipeRoot JavaDoc

@labkey-jeckels
Copy link
Contributor

labkey-jeckels commented Nov 11, 2025

I've added similar try/catch in a few places too. I wonder if we should automatically treat InvalidPathException -> NotFoundException more centrally, like in ExceptionUtil. It's a runtime exception so it's going to be hard to figure out all the places try/catch might be needed. @labkey-matthewb any opinions?

@labkey-matthewb
Copy link
Contributor

labkey-matthewb commented Nov 11, 2025

I've added similar try/catch in a few places too. I wonder if we should automatically treat InvalidPathException -> NotFoundException more centrally, like in ExceptionUtil. It's a runtime exception so it's going to be hard to figure out all the places try/catch might be needed. @labkey-matthewb any opinions?

Not really, I guess internally they are different (still not sure we care), but I don't have an opinion about consolidating how we report errors via APIs. If reporting fewer exceptions is better (or more consistent across OS) that seems fine.

@labkey-tchad labkey-tchad self-assigned this Nov 11, 2025
@labkey-jeckels
Copy link
Contributor

labkey-jeckels commented Nov 12, 2025

New approach pushed. @labkey-tchad @labkey-matthewb take a look at your convenience.

labkey-jeckels
labkey-jeckels approved these changes Nov 12, 2025
View reviewed changes
Copy link
Contributor

@labkey-jeckels labkey-jeckels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@labkey-tchad I'll go ahead and self-approve. Please take a look before merging

@labkey-tchad labkey-tchad changed the title Handle InvalidPathException in PipelinePathForm Respond with 404 for file references Nov 13, 2025
@labkey-tchad labkey-tchad changed the title Respond with 404 for file references Respond with 404 for invalid file references Nov 13, 2025
@labkey-tchad labkey-tchad merged commit 599edc7 into release25.11-SNAPSHOT Nov 13, 2025
10 checks passed
@labkey-tchad labkey-tchad deleted the 25.11_fb_invalidePipelinePath branch November 13, 2025 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@labkey-jeckels labkey-jeckels labkey-jeckels approved these changes

@labkey-matthewb labkey-matthewb Awaiting requested review from labkey-matthewb

Assignees

@labkey-jeckels labkey-jeckels

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@labkey-tchad @labkey-jeckels @labkey-matthewb